Conversation
e46319b to
26c751a
Compare
There was a problem hiding this comment.
Pull request overview
Adds a contract-first “compute sandbox” surface to void-control (bridge routes + CLI + multi-language SDKs + docs/examples) so downstream work can review and build against the API before the live void-box daemon integration lands.
Changes:
- Introduces compute schemas (
SandboxSpec,SnapshotSpec,SandboxPoolSpec) with validation + parsing helpers. - Adds bridge-managed routes for sandboxes/snapshots/pools and mock runtime support for sandbox lifecycle + exec.
- Adds CLI/SDK/tests/docs/examples for the compute surface (Rust tests, Python/Node/Go SDK clients, and docs/specs).
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/voidctl_execution_cli.rs | Adds CLI coverage for sandbox/snapshot/pool commands (stdin + interactive). |
| tests/sandbox_api.rs | Adds schema validation tests + bridge round-trip tests for compute routes. |
| src/sandbox/schema.rs | New YAML/JSON schemas + validation + parsers for sandbox/snapshot/pool specs. |
| src/sandbox/mod.rs | Exposes sandbox schema helpers/types publicly. |
| src/runtime/mod.rs | Adds sandbox lifecycle types + SandboxRuntime trait; stub impl for VoidBoxRuntimeClient. |
| src/runtime/mock.rs | Implements SandboxRuntime in MockRuntime for tests/mock behavior. |
| src/lib.rs | Exposes sandbox module behind serde feature. |
| src/bridge.rs | Adds compute routes + persistence for sandboxes/snapshots/pools in the bridge. |
| sdks/python/tests/test_client.py | Extends SDK tests to cover compute clients and error propagation. |
| sdks/python/src/void_control/snapshots.py | Adds snapshots subclient (create/get/list/replicate/delete). |
| sdks/python/src/void_control/sandboxes.py | Adds sandboxes subclient (create/get/list/exec/stop/delete). |
| sdks/python/src/void_control/pools.py | Adds pools subclient (create/get/scale). |
| sdks/python/src/void_control/models.py | Adds compute models (sandbox/snapshot/pool records + delete/exec results). |
| sdks/python/src/void_control/client.py | Wires compute subclients + adds delete_json. |
| sdks/python/examples/sandbox_create.py | Adds a Python example for sandbox creation. |
| sdks/python/README.md | Documents compute subclients + example. |
| sdks/node/test/client.test.mjs | Extends Node tests for compute routes + bridge error handling. |
| sdks/node/src/snapshots.js | Adds snapshots subclient. |
| sdks/node/src/sandboxes.js | Adds sandboxes subclient. |
| sdks/node/src/pools.js | Adds pools subclient. |
| sdks/node/src/models.js | Adds compute model mappers. |
| sdks/node/src/client.js | Wires compute subclients + adds deleteJson. |
| sdks/node/examples/sandboxCreate.mjs | Adds a Node example for sandbox creation. |
| sdks/node/README.md | Documents compute subclients + example. |
| sdks/go/snapshots.go | Adds snapshots client methods. |
| sdks/go/sandboxes.go | Adds sandboxes client methods. |
| sdks/go/pools.go | Adds pools client methods. |
| sdks/go/models.go | Adds compute models for Go SDK. |
| sdks/go/examples/sandbox_create/main.go | Adds a Go example for sandbox creation. |
| sdks/go/client_test.go | Extends Go tests for compute methods + error propagation. |
| sdks/go/client.go | Wires compute subclients + adds deleteJSON. |
| sdks/go/README.md | Documents compute surface + example. |
| examples/compute/snapshot-replicate.yaml | Adds snapshot replicate request example. |
| examples/compute/snapshot-from-sandbox.yaml | Adds snapshot create example. |
| examples/compute/sandbox-python.yaml | Adds sandbox spec example. |
| examples/compute/pool-scale.yaml | Adds pool scale request example. |
| examples/compute/pool-python.yaml | Adds pool spec example. |
| examples/README.md | Documents compute example payloads. |
| docs/superpowers/specs/2026-04-27-void-box-compute-daemon-prerequisites.md | Specifies daemon prerequisites for eventual live integration. |
| docs/superpowers/specs/2026-04-21-compute-sandbox-api-draft.md | Adds the compute API draft spec. |
| docs/superpowers/plans/2026-04-21-compute-sandbox-api-phase1-implementation.md | Adds phase-1 implementation plan. |
| README.md | Documents compute surface (HTTP/CLI/SDK usage) and limitations. |
| AGENTS.md | Documents compute surface boundary/limitations + endpoints/CLI commands. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let Some(sandbox_id) = path | ||
| .strip_prefix("/v1/sandboxes/") | ||
| .and_then(|rest| rest.strip_suffix("/exec")) | ||
| else { | ||
| return json_response( | ||
| 404, | ||
| &ApiError { | ||
| code: "NOT_FOUND", | ||
| message: format!("no route for POST {path}"), | ||
| retryable: false, | ||
| }, | ||
| ); | ||
| }; | ||
| let record = match load_sandbox_record(config, sandbox_id) { | ||
| Ok(record) => record, | ||
| Err(err) if err.kind() == std::io::ErrorKind::NotFound => { |
There was a problem hiding this comment.
handle_sandbox_exec extracts sandbox_id from the URL without validating it (unlike handle_sandbox_get/handle_sandbox_delete). Because sandbox_id is later used to build a filesystem path, IDs containing path separators (e.g. / and on Windows \\) or empty IDs can lead to unexpected file access / path traversal. Add the same guard as the GET/DELETE handlers (reject empty IDs and any sandbox_id containing path separators) before calling load_sandbox_record.
| let Some(sandbox_id) = path | ||
| .strip_prefix("/v1/sandboxes/") | ||
| .and_then(|rest| rest.strip_suffix("/stop")) | ||
| else { | ||
| return json_response( | ||
| 404, | ||
| &ApiError { | ||
| code: "NOT_FOUND", | ||
| message: format!("no route for POST {path}"), | ||
| retryable: false, | ||
| }, | ||
| ); | ||
| }; | ||
| let mut record = match load_sandbox_record(config, sandbox_id) { | ||
| Ok(record) => record, |
There was a problem hiding this comment.
handle_sandbox_stop does not validate sandbox_id extracted from the URL. Since sandbox_id is used to read/write JSON files under execution_dir, an ID containing path separators (or an empty ID) can cause unexpected filesystem access. Mirror the sandbox_id.contains('/') (and \\) + non-empty checks used in handle_sandbox_get/handle_sandbox_delete before loading/saving the record.
| let Some(snapshot_id) = path | ||
| .strip_prefix("/v1/snapshots/") | ||
| .and_then(|rest| rest.strip_suffix("/replicate")) | ||
| else { | ||
| return json_response( | ||
| 404, | ||
| &ApiError { | ||
| code: "NOT_FOUND", | ||
| message: format!("no route for POST {path}"), | ||
| retryable: false, | ||
| }, | ||
| ); | ||
| }; | ||
| let mut record = match load_snapshot_record(config, snapshot_id) { | ||
| Ok(record) => record, | ||
| Err(err) if err.kind() == std::io::ErrorKind::NotFound => { |
There was a problem hiding this comment.
handle_snapshot_replicate does not validate snapshot_id extracted from the URL before using it to load/write a JSON file. If snapshot_id contains path separators (or is empty), this can cause path traversal / unexpected filesystem access. Add the same non-empty + no path separators validation used in handle_snapshot_get/handle_snapshot_delete before calling load_snapshot_record.
| let Some(pool_id) = path | ||
| .strip_prefix("/v1/pools/") | ||
| .and_then(|rest| rest.strip_suffix("/scale")) | ||
| else { | ||
| return json_response( | ||
| 404, | ||
| &ApiError { | ||
| code: "NOT_FOUND", | ||
| message: format!("no route for GET {path}"), | ||
| message: format!("no route for POST {path}"), | ||
| retryable: false, | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| let response = handle_execution_get(&execution_path, config); | ||
| let Ok(mut value) = serde_json::from_slice::<Value>(&response.body) else { | ||
| return response; | ||
| }; | ||
| if response.status == 200 { | ||
| let Some(object) = value.as_object_mut() else { | ||
| return response; | ||
| }; | ||
| let execution_id = object | ||
| .get("execution") | ||
| .and_then(|execution| execution.get("execution_id")) | ||
| .cloned() | ||
| .unwrap_or(Value::Null); | ||
| object.insert("kind".to_string(), Value::String("batch".to_string())); | ||
| object.insert("run_id".to_string(), execution_id); | ||
| let request = match parse_pool_scale_request(body) { | ||
| Ok(request) => request, | ||
| Err(response) => return response, | ||
| }; | ||
| let mut record = match load_pool_record(config, pool_id) { | ||
| Ok(record) => record, | ||
| Err(err) if err.kind() == std::io::ErrorKind::NotFound => { | ||
| return json_response( | ||
| 404, |
There was a problem hiding this comment.
handle_pool_scale extracts pool_id from the URL but never validates it (unlike handle_pool_get). Since pool_id is used to read/write JSON files, a pool_id containing path separators (or an empty ID) can lead to unexpected filesystem access. Add a guard (reject empty IDs and any ID containing / or \\) before calling load_pool_record.
| if request.targets.is_empty() { | ||
| return Err(json_response( | ||
| 400, | ||
| &ApiError { | ||
| code: "INVALID_SNAPSHOT_REPLICATION", | ||
| message: "targets must not be empty".to_string(), | ||
| retryable: false, | ||
| }, | ||
| )); | ||
| } | ||
| Ok(request) |
There was a problem hiding this comment.
parse_snapshot_replicate_request validates targets is non-empty but does not validate that each entry is non-empty/whitespace (the schema validator for SnapshotSpec does). This allows persisting invalid target node IDs (e.g. ""). Add per-entry validation and return INVALID_SNAPSHOT_REPLICATION if any target is empty after trimming.
| @dataclass(slots=True) | ||
| class SnapshotRecord: | ||
| snapshot_id: str | ||
| source_sandbox_id: str | ||
| distribution: dict[str, Any] | ||
|
|
||
| @classmethod | ||
| def from_json(cls, payload: dict[str, Any]) -> "SnapshotRecord": | ||
| snapshot = payload["snapshot"] | ||
| return cls( | ||
| snapshot_id=str(snapshot["snapshot_id"]), | ||
| source_sandbox_id=str(snapshot.get("source_sandbox_id", "")), | ||
| distribution=dict(snapshot.get("distribution", {})), | ||
| ) |
There was a problem hiding this comment.
SnapshotRecord.from_json calls dict(snapshot.get("distribution", {})). If the API returns "distribution": null (which can happen because the Rust bridge serializes Option as null), dict(None) will raise a TypeError. Use snapshot.get("distribution") or {} (and similarly guard other dict(...) conversions) so the SDK tolerates null values.
| @dataclass(slots=True) | ||
| class SandboxDeleteResult: | ||
| kind: str | ||
| sandbox_id: str | ||
|
|
||
| @classmethod | ||
| def from_json(cls, payload: dict[str, Any]) -> "SandboxDeleteResult": | ||
| return cls( | ||
| kind=str(payload.get("kind", "")), | ||
| sandbox_id=str(payload.get("sandbox_id", "")), | ||
| ) | ||
|
|
||
|
|
||
| @dataclass(slots=True) | ||
| class SnapshotDeleteResult: | ||
| kind: str | ||
| snapshot_id: str | ||
|
|
||
| @classmethod | ||
| def from_json(cls, payload: dict[str, Any]) -> "SnapshotDeleteResult": | ||
| return cls( | ||
| kind=str(payload.get("kind", "")), | ||
| snapshot_id=str(payload.get("snapshot_id", "")), | ||
| ) |
There was a problem hiding this comment.
SandboxDeleteResult and SnapshotDeleteResult are defined twice in this module (the second definitions at the bottom overwrite the first ones). This makes the file harder to maintain and can mask future changes (e.g. if one definition is updated but not the other). Remove the duplicate class definitions and keep a single canonical definition for each model.
Problem
void-controlneeds a compute-oriented control-plane surface for sandboxes, snapshots, and prewarm pools, but the livevoid-boxdaemon support is not merged yet. We still need a reviewable contract, CLI, SDK, docs, and tests on the control-plane side.Summary of changes
sandboxes,snapshots, andpoolsvoid-controlvoidctlcompute commandsImportant boundary
feat/template-first-agent-apipoolremains avoid-controlresponsibilityVoidBoxRuntimeClientintegration is intentionally deferred until thevoid-boxdaemon exposes the required endpointsSpecs and docs
Verification
Follow-up
void-boxdaemon routes once the runtime-side work lands